Adapt channel balance reporting to use confirmed candidate#634
Adapt channel balance reporting to use confirmed candidate#634tnull merged 1 commit intolightningdevkit:developfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
| } => { | ||
| let balance = balance_candidates.get(confirmed_balance_candidate_index).unwrap(); | ||
|
|
||
| Self::ClaimableOnChannelClose { |
There was a problem hiding this comment.
Mind updating the docs on our ClaimableOnChannelClose to note that we're just exposing this balance in a splicing scenario?
There was a problem hiding this comment.
Added that values are based on the confirmed channel state, ignoring pending splices. I don't think it is what you suggest, that we are just exposing in a splicing scenario?
There was a problem hiding this comment.
Noted above that this is not the case. Confirmed but not locked splices are taken into account.
Also, LdkBalance::claimable_amount_satoshis uses a different way of calculating this when the index is 0. But maybe that is just to avoid the unwrap? (cc:: @wpaulino)
There was a problem hiding this comment.
Yeah when the index is 0, either we don't have a pending splice, or we do but none of the splice transactions has confirmed. In the latter case, we still want to report some indication of the unconfirmed splice balance.
With splicing now implemented, a channel may have multiple holder commitment transactions and corresponding balance candidates. ldk-node now reports the confirmed balance candidate rather than a single static balance, ensuring the exposed value matches the channel's onchain state. Other candidate balances remain internal for now.
7729e0c to
006a06e
Compare
| /// force-closed now. Values do not take into account any pending splices and are only based | ||
| /// on the confirmed state of the channel. |
There was a problem hiding this comment.
I don't believe this is the case. It will take into account a pending splice if it is confirmed but not locked (i.e., doesn't have sufficient number of confirmations).
There was a problem hiding this comment.
BTW, let us know if there is a way to update the docs to make this clear.
| } => { | ||
| let balance = balance_candidates.get(confirmed_balance_candidate_index).unwrap(); | ||
|
|
||
| Self::ClaimableOnChannelClose { |
There was a problem hiding this comment.
Noted above that this is not the case. Confirmed but not locked splices are taken into account.
Also, LdkBalance::claimable_amount_satoshis uses a different way of calculating this when the index is 0. But maybe that is just to avoid the unwrap? (cc:: @wpaulino)
With splicing now implemented, a channel may have multiple holder commitment transactions and corresponding balance candidates. ldk-node now reports the confirmed balance candidate rather than a single static balance, ensuring the exposed value matches the channel's onchain state. Other candidate balances remain internal for now.